Enable DH in generate_psa_tests.py #7909

mpg · 2023-07-11T08:31:40Z

Description

Resolves #7812: add support for DH keys to generate_psa_tests.py.

PR checklist

changelog not required - tests only
backport not required - we only have that feature in development
tests provided - that's what the PR is about :)

AndrzejKurek · 2023-07-11T09:09:34Z

tests/scripts/generate_psa_tests.py


    def test_cases_for_not_supported(self) -> Iterator[test_case.TestCase]:
        """Generate test cases that exercise the creation of keys of unsupported types."""
        for key_type in sorted(self.constructors.key_types):
            if key_type in self.ECC_KEY_TYPES:
                continue
+            if key_type in self.DH_KEY_TYPES:


Minor: this could be done in one line with the previous if.

I prefer separate lines. This way you can set a breakpoint on just one of them.

AndrzejKurek

Looks good comparing to the way ECC_KEY_TYPES are handled.

gilles-peskine-arm

A couple of minor issues. Also I haven't ensured that test_suite_psa_crypto_not_supported.generated.data looks right because of #7915.

scripts/mbedtls_dev/crypto_knowledge.py

tests/scripts/generate_psa_tests.py

gilles-peskine-arm · 2023-07-18T09:49:00Z

CI is not happy.

mpg · 2023-07-18T10:04:30Z

Just pushed a new version that I think fixes #7915. It won't pass CI yet as make test doesn't pass in the default config: PSA key_agreement FFDH: incompatible with DH_KEY_PAIR(RFC7919) from test_suite_psa_crypto_op_fail.generated is failing and I need to look into this, but I wanted to push the current state in order to share the fix for #7915.

gilles-peskine-arm · 2023-07-18T10:33:57Z

PSA key_agreement FFDH: incompatible with DH_KEY_PAIR(RFC7919) from test_suite_psa_crypto_op_fail.generated is failing

That means in crypto_knowledge.py, in KeyType, can_do isn't doing its job. And indeed it's missing a case for FFDH.

mpg · 2023-07-19T10:00:16Z

CI came green, and the number of test cases that are never executed is down to something more reasonable - in particular, the list does not include any test from any generated.data file. So, I think this is ready for review.

gilles-peskine-arm

4325774 looks good to me, but now generate_psa_tests.py no longer needs the MBEDTLS_GENPRIME insertion hack.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>

- RSA was missing the MBEDTLS_ prefix. - DH needs the same temporary fix (prefix + suffix) for now. - hack_dependencies_not_implemented() needs to ignore MBEDTLS_PSA_WANT dependencies. While at it, make the code currently used for ECC more generic, so that it's ready to be used for RSA and DH in the near future. Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>

mpg · 2023-07-26T07:42:36Z

Indeed, it became unnecessary 15h ago when #7902 was merged. I've rebased to fix the conflict and remove the last commit.

gilles-peskine-arm

Looks good to me, except for wrong dependencies in test_suite_psa_crypto_not_supported.generated.data which is a pre-existing issue that will be resolved with #7915.

mpg · 2023-07-26T13:12:37Z

Well, I did try to resolve #7915 with this PR (as noted in the PR description), specifically this commit (whose message is out-of-date after rebasing btw). What did I miss?

gilles-peskine-arm · 2023-07-26T13:26:10Z

Well, I did try to resolve #7915 with this PR (as noted in the PR description), specifically this commit (whose message is out-of-date after rebasing btw). What did I miss?

Oops, I missed that. Well, #7915 is not resolved, but I think the current state is good to merge anyway as long as we don't close the issue.

Observation:

# Take config-suite-b.h plus PSA (which is executed by test-ref-configs.pl on the CI)
mbedtls-prepare-build -p debug -d build-psa-suite-b --config-file=configs/config-suite-b.h --config-set=MBEDTLS_PSA_CRYPTO_C,MBEDTLS_USE_PSA_CRYPTO
make -C build-psa-suite-b tests/test_suite_psa_crypto_not_supported.generated.run

Comparing the output on development and this PR:

diff ../../f1c032adba124fecbb43ff52d3e08f7157ae69e1.txt ../../182eb1514e49a1dc1eafffd83ab7abf03e3c6591.txt
1c1
< make: Entering directory '/home/gilpes01/z/dev/mbedtls/main/build-psa-suite-b'
---
> make: Entering directory '/home/gilpes01/z/dev/mbedtls/reviewing/7909-dh-generate-psa-tests/build-psa-suite-b'
224a225,254
> PSA import DH_KEY_PAIR(RFC7919) 2048-bit type not supported ....... ----
> PSA generate DH_KEY_PAIR(RFC7919) 2048-bit type not supported ..... ----
> PSA import DH_KEY_PAIR(RFC7919) 3072-bit type not supported ....... ----
> PSA generate DH_KEY_PAIR(RFC7919) 3072-bit type not supported ..... ----
> PSA import DH_KEY_PAIR(RFC7919) 4096-bit type not supported ....... ----
> PSA generate DH_KEY_PAIR(RFC7919) 4096-bit type not supported ..... ----
> PSA import DH_KEY_PAIR(RFC7919) 6144-bit type not supported ....... ----
> PSA generate DH_KEY_PAIR(RFC7919) 6144-bit type not supported ..... ----
> PSA import DH_KEY_PAIR(RFC7919) 8192-bit type not supported ....... ----
> PSA generate DH_KEY_PAIR(RFC7919) 8192-bit type not supported ..... ----
> PSA import DH_KEY_PAIR(RFC7919) 2048-bit group not supported ...... ----
> PSA generate DH_KEY_PAIR(RFC7919) 2048-bit group not supported .... ----
> PSA import DH_KEY_PAIR(RFC7919) 3072-bit group not supported ...... ----
> PSA generate DH_KEY_PAIR(RFC7919) 3072-bit group not supported .... ----
> PSA import DH_KEY_PAIR(RFC7919) 4096-bit group not supported ...... ----
> PSA generate DH_KEY_PAIR(RFC7919) 4096-bit group not supported .... ----
> PSA import DH_KEY_PAIR(RFC7919) 6144-bit group not supported ...... ----
> PSA generate DH_KEY_PAIR(RFC7919) 6144-bit group not supported .... ----
> PSA import DH_KEY_PAIR(RFC7919) 8192-bit group not supported ...... ----
> PSA generate DH_KEY_PAIR(RFC7919) 8192-bit group not supported .... ----
> PSA import DH_PUBLIC_KEY(RFC7919) 2048-bit type not supported ..... ----
> PSA import DH_PUBLIC_KEY(RFC7919) 3072-bit type not supported ..... ----
> PSA import DH_PUBLIC_KEY(RFC7919) 4096-bit type not supported ..... ----
> PSA import DH_PUBLIC_KEY(RFC7919) 6144-bit type not supported ..... ----
> PSA import DH_PUBLIC_KEY(RFC7919) 8192-bit type not supported ..... ----
> PSA import DH_PUBLIC_KEY(RFC7919) 2048-bit group not supported .... ----
> PSA import DH_PUBLIC_KEY(RFC7919) 3072-bit group not supported .... ----
> PSA import DH_PUBLIC_KEY(RFC7919) 4096-bit group not supported .... ----
> PSA import DH_PUBLIC_KEY(RFC7919) 6144-bit group not supported .... ----
> PSA import DH_PUBLIC_KEY(RFC7919) 8192-bit group not supported .... ----
228,229c258,259
< PASSED (222 / 222 tests (169 skipped))
< make: Leaving directory '/home/gilpes01/z/dev/mbedtls/main/build-psa-suite-b'
---
> PASSED (252 / 252 tests (199 skipped))
> make: Leaving directory '/home/gilpes01/z/dev/mbedtls/reviewing/7909-dh-generate-psa-tests/build-psa-suite-b'

Resolving 7915 would mean that the DH tests pass here, and also the ECC test cases for curves that are not supported in development pass here. But they're still skipped.

tom-daubney-arm

LGTM, thanks.

gilles-peskine-arm · 2023-07-26T17:45:43Z

I've removed the statement about #7915 from the issue description since I approved thinking that this would resolve #7815 but not #7915.

mpg added enhancement priority-high High priority - will be reviewed soon labels Jul 11, 2023

mpg self-assigned this Jul 11, 2023

gilles-peskine-arm self-requested a review July 11, 2023 08:38

AndrzejKurek reviewed Jul 11, 2023

View reviewed changes

AndrzejKurek previously approved these changes Jul 11, 2023

View reviewed changes

gilles-peskine-arm requested changes Jul 11, 2023

View reviewed changes

scripts/mbedtls_dev/crypto_knowledge.py Show resolved Hide resolved

tests/scripts/generate_psa_tests.py Show resolved Hide resolved

gilles-peskine-arm added needs-work size-s Estimated task size: small (~2d) labels Jul 11, 2023

mpg dismissed AndrzejKurek’s stale review via d4bd05c July 18, 2023 09:04

mpg added the needs-ci Needs to pass CI tests label Jul 18, 2023

mpg force-pushed the dh-generate-psa-tests branch from d4bd05c to a1f14f2 Compare July 18, 2023 10:01

mpg mentioned this pull request Jul 18, 2023

Incorrect dependencies for key pairs in test_suite_psa_crypto_not_supported.generated.data #7915

Open

mpg requested a review from gilles-peskine-arm July 19, 2023 10:00

mpg added needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review and removed needs-work needs-ci Needs to pass CI tests labels Jul 19, 2023

gilles-peskine-arm reviewed Jul 26, 2023

View reviewed changes

gilles-peskine-arm added needs-work and removed needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels Jul 26, 2023

mpg added 2 commits July 26, 2023 09:32

Enable DH in generate_psa_tests.py

afe4b79

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>

Shorten DH_FAMILY just like ECC_FAMILY

636d857

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>

mpg added 2 commits July 26, 2023 09:34

Fix KeyType.can_do() for DH+FFDH

182eb15

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>

mpg force-pushed the dh-generate-psa-tests branch from d0daaee to 182eb15 Compare July 26, 2023 07:42

valeriosetti mentioned this pull request Jul 26, 2023

Define PSA_WANT_xxx_KEY_PAIR_yyy step 2/DH #7903

Merged

3 tasks

mpg requested review from tom-daubney-arm and gilles-peskine-arm July 26, 2023 07:45

mpg added needs-review Every commit must be reviewed by at least two team members, and removed needs-work labels Jul 26, 2023

gilles-peskine-arm approved these changes Jul 26, 2023

View reviewed changes

tom-daubney-arm approved these changes Jul 26, 2023

View reviewed changes

gilles-peskine-arm added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, labels Jul 26, 2023

gilles-peskine-arm added this pull request to the merge queue Jul 26, 2023

Merged via the queue into Mbed-TLS:development with commit 51ed313 Jul 26, 2023
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable DH in generate_psa_tests.py #7909

Enable DH in generate_psa_tests.py #7909

mpg commented Jul 11, 2023 •

edited by gilles-peskine-arm

Loading

AndrzejKurek Jul 11, 2023

gilles-peskine-arm Jul 11, 2023

AndrzejKurek left a comment

gilles-peskine-arm left a comment

gilles-peskine-arm commented Jul 18, 2023

mpg commented Jul 18, 2023

gilles-peskine-arm commented Jul 18, 2023

mpg commented Jul 19, 2023

gilles-peskine-arm left a comment

mpg commented Jul 26, 2023

gilles-peskine-arm left a comment

mpg commented Jul 26, 2023

gilles-peskine-arm commented Jul 26, 2023

tom-daubney-arm left a comment

gilles-peskine-arm commented Jul 26, 2023

Enable DH in generate_psa_tests.py #7909

Enable DH in generate_psa_tests.py #7909

Conversation

mpg commented Jul 11, 2023 • edited by gilles-peskine-arm Loading

Description

PR checklist

AndrzejKurek Jul 11, 2023

Choose a reason for hiding this comment

gilles-peskine-arm Jul 11, 2023

Choose a reason for hiding this comment

AndrzejKurek left a comment

Choose a reason for hiding this comment

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

gilles-peskine-arm commented Jul 18, 2023

mpg commented Jul 18, 2023

gilles-peskine-arm commented Jul 18, 2023

mpg commented Jul 19, 2023

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

mpg commented Jul 26, 2023

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

mpg commented Jul 26, 2023

gilles-peskine-arm commented Jul 26, 2023

tom-daubney-arm left a comment

Choose a reason for hiding this comment

gilles-peskine-arm commented Jul 26, 2023

mpg commented Jul 11, 2023 •

edited by gilles-peskine-arm

Loading